Apple: Fix AF_SYSTEM and AF_SYS_CONTROL #3047

siegfried · 2022-12-23T10:55:41Z

Trying to fix the issue #3044

Here is the definition I found:

/*!
 *       @struct sockaddr_ctl
 *       @discussion The controller address structure is used to establish
 *               contact between a user client and a kernel controller. The
 *               sc_id/sc_unit uniquely identify each controller. sc_id is a
 *               unique identifier assigned to the controller. The identifier can
 *               be assigned by the system at registration time or be a 32-bit
 *               creator code obtained from Apple Computer. sc_unit is a unit
 *               number for this sc_id, and is privately used by the kernel
 *               controller to identify several instances of the controller.
 *       @field sc_len The length of the structure.
 *       @field sc_family AF_SYSTEM.
 *       @field ss_sysaddr AF_SYS_KERNCONTROL.
 *       @field sc_id Controller unique identifier.
 *       @field sc_unit Kernel controller private unit number.
 *       @field sc_reserved Reserved, must be set to zero.
 */
struct sockaddr_ctl {
	u_char      sc_len;     /* depends on size of bundle ID string */
	u_char      sc_family;  /* AF_SYSTEM */
	u_int16_t   ss_sysaddr; /* AF_SYS_KERNCONTROL */
	u_int32_t   sc_id;      /* Controller unique identifier  */
	u_int32_t   sc_unit;    /* Developer private unit number */
	u_int32_t   sc_reserved[5];
};

Shall I alias AF_SYS_CONTROL to AF_SYS_KERNCONTROL?

rustbot · 2022-12-23T10:55:45Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information.

siegfried · 2023-01-08T20:59:15Z

src/unix/bsd/apple/mod.rs

@@ -3606,7 +3606,7 @@ pub const PF_ISDN: ::c_int = AF_ISDN;
 pub const PF_KEY: ::c_int = pseudo_AF_KEY;
 pub const PF_INET6: ::c_int = AF_INET6;
 pub const PF_NATM: ::c_int = AF_NATM;
-pub const PF_SYSTEM: ::c_int = AF_SYSTEM;
+pub const PF_SYSTEM: ::c_int = 32;


I changed the type of PF_SYSTEM to c_int regarding the man page of SOCKET(2):

NAME socket – create an endpoint for communication SYNOPSIS #include <sys/socket.h> int socket(int domain, int type, int protocol); DESCRIPTION socket() creates an endpoint for communication and returns a descriptor. The domain parameter specifies a communications domain within which communication will take place; this selects the protocol family which should be used. These families are defined in the include file ⟨sys/socket.h⟩. The currently understood formats are PF_LOCAL Host-internal protocols, formerly called PF_UNIX, PF_UNIX Host-internal protocols, deprecated, use PF_LOCAL, PF_INET Internet version 4 protocols, PF_ROUTE Internal Routing protocol, PF_KEY Internal key-management function, PF_INET6 Internet version 6 protocols, PF_SYSTEM System domain, PF_NDRV Raw access to network device, PF_VSOCK VM Sockets protocols The socket has the indicated type, which specifies the semantics of communication. Currently defined types are: SOCK_STREAM SOCK_DGRAM SOCK_RAW A SOCK_STREAM type provides sequenced, reliable, two-way connection based byte streams. An out-of-band data transmission mechanism may be supported. A SOCK_DGRAM socket supports datagrams (connectionless, unreliable messages of a fixed (typically small) maximum length). SOCK_RAW sockets provide access to internal network protocols and interfaces. The type SOCK_RAW, which is available only to the super-user.

Just change to AF_SYSTEM as c_int rather than duplicating the literal

siegfried · 2023-01-08T21:05:54Z

src/unix/bsd/apple/mod.rs

 pub const AF_NETBIOS: ::c_int = 33;
 pub const AF_PPP: ::c_int = 34;
 pub const pseudo_AF_HDRCMPLT: ::c_int = 35;
-pub const AF_SYS_CONTROL: ::c_int = 2;
+pub const AF_SYS_CONTROL: ::c_ushort = 2;


The old examples uses the value of AF_SYS_CONTROL for AF_SYS_KERNCONTROL. I can also add AF_SYS_KERNCONTROL and leave AF_SYS_CONTROL untouched.

siegfried · 2023-02-04T09:34:05Z

Is there anything else needed from me?

JohnTitor · 2023-02-09T08:02:33Z

Changing types is a breaking change and we don't ship it on minor releases. I'll revisit once we decide to do a major release.

tgross35 · 2024-11-20T22:30:38Z

We have an upcoming major release where we can do breaking changes.

It seems unusual to only change one AF_* type - should they all be changed?

tgross35 · 2025-02-22T20:26:49Z

@rustbot author for the above question

siegfried · 2025-02-23T20:02:20Z

Updated. Sorry for the delay. Please review.

tgross35 · 2025-02-23T21:19:34Z

Just to clarify; is sc_family only ever set to AF_SYSTEM and ss_sysaddr is only ever AF_SYS_CONTROL? Meaning no other constants are ever used for these fields.

tgross35 · 2025-02-23T21:20:13Z

src/unix/bsd/apple/mod.rs

 pub const AF_NETBIOS: c_int = 33;
 pub const AF_PPP: c_int = 34;
 pub const pseudo_AF_HDRCMPLT: c_int = 35;
 pub const AF_IEEE80211: c_int = 37;
 pub const AF_UTUN: c_int = 38;
 pub const AF_VSOCK: c_int = 40;
-pub const AF_SYS_CONTROL: c_int = 2;
+pub const AF_SYS_CONTROL: c_ushort = 2;


Suggested change

pub const AF_SYS_CONTROL: c_ushort = 2;

pub const AF_SYS_CONTROL: c_ushort = 2;

It looks like the field is u_int16_t, so this can just be u16 https://developer.apple.com/documentation/kernel/sockaddr_ctl/1809252-ss_sysaddr.

Updated. Waiting on CI.

The CI test failed after I change AF_SYS_CONTROL to u16. Any idea why? @tgross35

It doesn't look related, the android CI has been flaky.

siegfried · 2025-02-23T21:41:18Z

Just to clarify; is sc_family only ever set to AF_SYSTEM and ss_sysaddr is only ever AF_SYS_CONTROL? Meaning no other constants are ever used for these fields.

I think so regarding to the document.

tgross35

LGTM, please squash. I think the CI failure is unrelated.

Since this is breaking:

@rustbot label +stable-declined

siegfried · 2025-02-23T21:59:11Z

Squashed.

rustbot assigned JohnTitor Dec 23, 2022

rustbot added the S-waiting-on-review label Dec 23, 2022

JohnTitor added breakage-candidate S-blocked and removed S-waiting-on-review labels Dec 23, 2022

siegfried mentioned this pull request Dec 25, 2022

Support macOS siegfried/utuntap#28

Merged

siegfried force-pushed the fix-const-type branch from 27e5f21 to 6761b10 Compare January 8, 2023 20:54

siegfried commented Jan 8, 2023

View reviewed changes

tgross35 added the S-waiting-on-review label Nov 20, 2024

rustbot added S-waiting-on-author and removed S-waiting-on-review S-blocked labels Feb 22, 2025

siegfried force-pushed the fix-const-type branch from 6761b10 to 624a424 Compare February 23, 2025 19:40

rustbot added O-macos O-unix labels Feb 23, 2025

tgross35 changed the title ~~Fix AF_SYSTEM and AF_SYS_CONTROL~~ Apple: Fix AF_SYSTEM and AF_SYS_CONTROL Feb 23, 2025

tgross35 reviewed Feb 23, 2025

View reviewed changes

tgross35 approved these changes Feb 23, 2025

View reviewed changes

rustbot added the stable-declined This change is breaking, difficult to backport, low priority, or otherwise not relevant for 0.2 label Feb 23, 2025

siegfried force-pushed the fix-const-type branch from 6a26836 to a6ff0dd Compare February 23, 2025 21:58

Fix AF_SYSTEM and AF_SYS_CONTROL

92bb592

tgross35 force-pushed the fix-const-type branch from a6ff0dd to 92bb592 Compare February 26, 2025 05:41

tgross35 enabled auto-merge February 26, 2025 05:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apple: Fix AF_SYSTEM and AF_SYS_CONTROL #3047

Apple: Fix AF_SYSTEM and AF_SYS_CONTROL #3047

siegfried commented Dec 23, 2022 •

edited

Loading

rustbot commented Dec 23, 2022

siegfried Jan 8, 2023

tgross35 Nov 20, 2024

siegfried Jan 8, 2023

siegfried commented Feb 4, 2023

JohnTitor commented Feb 9, 2023

tgross35 commented Nov 20, 2024 •

edited

Loading

tgross35 commented Feb 22, 2025

siegfried commented Feb 23, 2025

tgross35 commented Feb 23, 2025 •

edited

Loading

tgross35 Feb 23, 2025

siegfried Feb 23, 2025

siegfried Feb 23, 2025

tgross35 Feb 23, 2025

siegfried commented Feb 23, 2025

tgross35 left a comment •

edited

Loading

siegfried commented Feb 23, 2025

	pub const AF_SYS_CONTROL: c_ushort = 2;
	pub const AF_SYS_CONTROL: c_ushort = 2;

Apple: Fix AF_SYSTEM and AF_SYS_CONTROL #3047

Are you sure you want to change the base?

Apple: Fix AF_SYSTEM and AF_SYS_CONTROL #3047

Conversation

siegfried commented Dec 23, 2022 • edited Loading

rustbot commented Dec 23, 2022

siegfried Jan 8, 2023

Choose a reason for hiding this comment

tgross35 Nov 20, 2024

Choose a reason for hiding this comment

siegfried Jan 8, 2023

Choose a reason for hiding this comment

siegfried commented Feb 4, 2023

JohnTitor commented Feb 9, 2023

tgross35 commented Nov 20, 2024 • edited Loading

tgross35 commented Feb 22, 2025

siegfried commented Feb 23, 2025

tgross35 commented Feb 23, 2025 • edited Loading

tgross35 Feb 23, 2025

Choose a reason for hiding this comment

siegfried Feb 23, 2025

Choose a reason for hiding this comment

siegfried Feb 23, 2025

Choose a reason for hiding this comment

tgross35 Feb 23, 2025

Choose a reason for hiding this comment

siegfried commented Feb 23, 2025

tgross35 left a comment • edited Loading

Choose a reason for hiding this comment

siegfried commented Feb 23, 2025

siegfried commented Dec 23, 2022 •

edited

Loading

tgross35 commented Nov 20, 2024 •

edited

Loading

tgross35 commented Feb 23, 2025 •

edited

Loading

tgross35 left a comment •

edited

Loading